Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hey-hammad
left a comment
There was a problem hiding this comment.
Nice effort, one minor issues needs fixing. Thanks
| // Finally, correct the code to fix the problem | ||
| // =============> write your new code here | ||
|
|
||
| function convertToPercentage() { |
There was a problem hiding this comment.
this fixes the issue, however the does the function still works correctly?
There was a problem hiding this comment.
Think about, when given an input can the does the function return the correct output?
There was a problem hiding this comment.
The function wasn’t producing the correct output, but I’ve corrected it now. Thank you
abdishakoor-dev
left a comment
There was a problem hiding this comment.
convertToPercentage works now for any input, thanks. Your time-format answers are all correct too.
A few things before I can mark this Complete:
-
1-key-errors/0.js: line 14 does not match your new code. -
1-key-errors/1.jsline 23 and2-mandatory-debug/2.jsline 34: see my comments there. -
1-key-errors/2.js: line 14 is still empty. -
2-cases.js: see my comment on the function name. -
Formatting. "My code is consistently formatted" is on the checklist. The tool that does it is called Prettier. It fixes spacing and indentation to one agreed style. Then a reviewer only sees the changes you meant to make. At the moment all 10 of your files fail that check.
Prettier comes with the CYF extension pack from onboarding. Not sure you have it? In VS Code, go to Extensions and search for CodeYourFuture Extension Pack: https://marketplace.visualstudio.com/items?itemName=CodeYourFuture.cyf-extension-pack
Then open each file you changed. Right click in the editor and choose Format Document. Pick Prettier if VS Code asks. Save and commit. To make this happen every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md
Add the Needs Review label again once you've pushed.
| // } | ||
|
|
||
| // =============> write your explanation here | ||
| //- I changed the variable name to "strg" to avoid the error. |
There was a problem hiding this comment.
Your code on line 17 doesn't use strg any more. Does line 14 still describe your fix? Update it to match.
There was a problem hiding this comment.
updated the explanation on line 14.
There was a problem hiding this comment.
Line 14 matches your code now. Good.
|
|
||
| // =============> write your new code here | ||
|
|
||
| const decimalNumber = 0.5; |
There was a problem hiding this comment.
Line 27 uses decimalNumber. Which one does it use, line 23 or the parameter on line 25? Is line 23 needed? If not, remove it.
There was a problem hiding this comment.
It uses the parameter defined on line 25. Since we’re not using the previously declared value of 0.5, I removed it as it was unnecessary.
There was a problem hiding this comment.
Right, it uses the parameter. Fixed.
| //-The function was looking at the outside "num" but Now there are two (num) one outside the scope "const num= 103" and i added another inside the function scope. | ||
| //-The function calculates something and returns it then console.log outside receives that returned value and prints it. | ||
|
|
||
| const num = 103; |
There was a problem hiding this comment.
Is num on line 34 used now? Your function uses its own num parameter. If line 34 is not needed, remove it.
There was a problem hiding this comment.
I removed the unnecessary const num = 103 because the function already receives num as a parameter.
| // return num * num; | ||
| // } | ||
|
|
||
| // =============> write the error message here |
There was a problem hiding this comment.
This answer is still empty. Your prediction on line 8 says SyntaxError. Was that right? Write the error name here.
There was a problem hiding this comment.
That's the one. Fixed.
| // Use the MDN string documentation to help you find a solution | ||
| // This might help https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/toUpperCase | ||
|
|
||
| function convertToUpperCase(sentence) { |
There was a problem hiding this comment.
Function names should tell others what the function does. If you use toUpperCase() as an inspiration, what would be a better name for this function? Remember you have been asked for a function to convert yo upper snake case, not just upper case. There is a difference.
There was a problem hiding this comment.
Changed to " toUpperSnakeCase " which I think is more descriptive and closely related to what the function does.
ec68ca5 to
a4a378f
Compare
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Everything on the list is done, and every file passes Prettier now. Marking this Complete. Well done.


Self checklist
Task code
CYF-1053
Changelist
Completed Sprint 3 tasks including time formatting, BMI calculation, toPounds, getLastDigit, and square functions.
Also fixed return statements, naming conventions, and documented the errors, predictions, and solutions.